avatar: Show placeholder on image load error #1882

vinodsingh-dev-gsoc · 2025-09-30T15:06:00Z

Fixes #1558.
This change improves the user experience by displaying a consistent placeholder when an avatar image fails to load from its URL.
Adds an errorBuilder to the AvatarImage widget and includes a test case to verify the new behavior.
Co-authored-by: Chris Bobbe [email protected]

chrisbobbe · 2025-10-01T17:56:30Z

Thanks for the PR! Please post before/after screenshots of the change. Please also change the commit sequence so the tests are added in the same commit that adds the behavior being tested.

vinodsingh-dev-gsoc · 2025-10-02T07:37:04Z

Hi chrisbobbe! Thanks a lot for the detailed feedback. I'm currently working on your suggestions. I've gathered the before/after screenshots and am now in the process of squashing the commits and fixing the test suite to make the PR perfect. I'll push the updated changes shortly! 😊

vinodsingh-dev-gsoc · 2025-10-03T15:14:39Z

status	CombinedScreen	DM List	Profile Screen
Before
After fix

As you can see in the "After" screenshot, the fix ensures that a placeholder is now displayed whenever an avatar URL is null or fails to load, instead of showing a blank space. This provides a clearer and more consistent UI for the user.

chrisbobbe

Thanks! Small comments below.

test/widgets/user_test.dart

lib/widgets/user.dart

chrisbobbe · 2025-10-03T22:10:36Z

Also please update the commit message to mention the issue being fixed, according to our style.

Co-authored-by: Chris Bobbe <[email protected]>

chrisbobbe · 2025-10-07T20:24:56Z

I see you've pushed some changes, but I'm not sure if you intend them to be ready for review; I assume you'll comment here when ready. 🙂

vinodsingh-dev-gsoc · 2025-10-08T01:36:57Z

Hi @chrisbobbe,

Thanks for the ping, and my apologies for the delay in updating you here.

I've addressed your suggestions and pushed the commits a few days ago. The PR should now be ready for another look.

Eager to hear your feedback. Thank you for your guidance!

alya · 2025-10-08T05:07:44Z

Please see our guide on how to demonstrate visual changes: https://zulip.readthedocs.io/en/latest/contributing/reviewable-prs.html#demonstrating-visual-changes. The before/after screenshots should be minimally different.

I would also like to see both placeholders and regular avatars in the same screenshot, and dark theme as well as light.

vinodsingh-dev-gsoc · 2025-10-08T06:34:05Z

Please see our guide on how to demonstrate visual changes: https://zulip.readthedocs.io/en/latest/contributing/reviewable-prs.html#demonstrating-visual-changes. The before/after screenshots should be minimally different.

I would also like to see both placeholders and regular avatars in the same screenshot, and dark theme as well as light.

Hi @alya, thanks for the detailed feedback! That makes perfect sense. I will generate the new screenshots with both themes and mixed avatars as you suggested and will post them here shortly.

vinodsingh-dev-gsoc · 2025-10-08T07:27:12Z

Before (Bot avatar URL fails)	After Fix (Shows Placeholder)

vinodsingh-dev-gsoc · 2025-10-15T11:03:22Z

Hi @alya and @chrisbobbe,

Thanks for the detailed feedback! I've now updated the PR description with the new screenshots to better demonstrate the visual changes.

As requested, I've included before/after images for both light and dark themes, which show both regular avatars and the new placeholders in the same view.

I believe this addresses all the points. Eager for your review when you have a moment. Thank you!

gnprice · 2025-10-16T00:29:01Z

Thanks. Please see our guidelines on a clean Git history (linked from this repo's README):
https://zulip.readthedocs.io/en/latest/contributing/commit-discipline.html

and revise these commits to follow those guidelines.

chrisbobbe

Thanks! Comments below, and please also address Greg's feedback about the commit sequence.

chrisbobbe · 2025-10-23T23:26:09Z

lib/widgets/user.dart

@@ -1,5 +1,4 @@
 import 'package:flutter/material.dart';
-


Please don't delete this blank line; it separates third-party imports from our own imports. (Here and elsewhere in the PR.)

chrisbobbe · 2025-10-23T23:26:54Z

lib/widgets/user.dart

      avatarUrl.get(physicalSize),
      filterQuality: FilterQuality.medium,
      fit: BoxFit.cover,
+      errorBuilder: (_, _, _) => _AvatarPlaceholder(size: size),


Please also change the resolvedUrl == null case to return _AvatarPlaceholder, too, for this part of the issue description in #1558:

because the URL is invalid

and add a test for that.

Same for the user == null case above that, for this part of the issue description:

Or, pre-#1556, for a message from an unknown sender

chrisbobbe · 2025-10-23T23:27:32Z

test/test_images.dart

+  }
  return httpClient;
 }
-


Keep this blank line, which exists to give the code some breathing room so it's more readable.

chrisbobbe · 2025-10-23T23:30:57Z

test/widgets/user_test.dart

+      await tester.pumpWidget(TestZulipApp(
+        child: PerAccountStoreWidget(
+          accountId: eg.selfAccount.id,
+          child: AvatarImage(userId: badUser.userId, size: 30))));


Should be able to pass an accountId to TestZulipApp instead of adding a PerAccountStoreWidget here.

chrisbobbe · 2025-10-23T23:31:36Z

test/widgets/user_test.dart

+    testWidgets('shows placeholder when image URL gives error', (WidgetTester tester) async {
+      addTearDown(testBinding.reset);
+      prepareBoringImageHttpClient(success: false);
+      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+      final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
+      final badUser = eg.user(avatarUrl: 'https://zulip.com/avatarinvalid.png');
+      await store.addUser(badUser);
+      await tester.pumpWidget(TestZulipApp(
+        child: PerAccountStoreWidget(
+          accountId: eg.selfAccount.id,
+          child: AvatarImage(userId: badUser.userId, size: 30))));
+      await tester.pumpAndSettle();
+      expect(
+        find.descendant(
+          of: find.byType(AvatarImage),
+          matching: find.byIcon(ZulipIcons.person),
+        ),
+        findsOneWidget,
+      );
+  debugNetworkImageHttpClientProvider = null;
+    });


nits:

Empty line before a test

Try to adjust indentation and spacing to follow the pattern of existing tests

Use check instead of expect; see existing tests for examples

vinodsingh-dev-gsoc force-pushed the fix/avatar-placeholder-1558 branch from 60b16e2 to 97f92e7 Compare October 1, 2025 16:17

vinodsingh-dev-gsoc force-pushed the fix/avatar-placeholder-1558 branch from 97f92e7 to 3f69209 Compare October 2, 2025 06:36

vinodsingh-dev-gsoc force-pushed the fix/avatar-placeholder-1558 branch from 3f69209 to 0b69fea Compare October 3, 2025 14:04

avatar: Show placeholder on image load error

aa123be

vinodsingh-dev-gsoc force-pushed the fix/avatar-placeholder-1558 branch from 0b69fea to aa123be Compare October 3, 2025 14:27

chrisbobbe reviewed Oct 3, 2025

View reviewed changes

test/widgets/user_test.dart Outdated Show resolved Hide resolved

test/widgets/user_test.dart Outdated Show resolved Hide resolved

test/widgets/user_test.dart Outdated Show resolved Hide resolved

lib/widgets/user.dart Outdated Show resolved Hide resolved

gnprice added the maintainer review PR ready for review by Zulip maintainers label Oct 3, 2025

vinodsingh-dev-gsoc and others added 3 commits October 4, 2025 05:22

Apply suggestion from @chrisbobbe

57ce7f1

Co-authored-by: Chris Bobbe <[email protected]>

Apply suggestion from @chrisbobbe

97c936d

Co-authored-by: Chris Bobbe <[email protected]>

avatar: Show placeholder on image load error

3ed8e3c

gnprice mentioned this pull request Oct 8, 2025

Show pronouns in @-mention autocomplete #313

Open

vinodsingh-dev-gsoc requested a review from chrisbobbe October 9, 2025 03:26

chrisbobbe reviewed Oct 23, 2025

View reviewed changes

avatar: Show placeholder on image load error #1882

Are you sure you want to change the base?

avatar: Show placeholder on image load error #1882

Uh oh!

Conversation

vinodsingh-dev-gsoc commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe commented Oct 1, 2025

Uh oh!

vinodsingh-dev-gsoc commented Oct 2, 2025

Uh oh!

vinodsingh-dev-gsoc commented Oct 3, 2025

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chrisbobbe commented Oct 3, 2025

Uh oh!

chrisbobbe commented Oct 7, 2025

Uh oh!

vinodsingh-dev-gsoc commented Oct 8, 2025

Uh oh!

alya commented Oct 8, 2025

Uh oh!

vinodsingh-dev-gsoc commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vinodsingh-dev-gsoc commented Oct 8, 2025

Uh oh!

vinodsingh-dev-gsoc commented Oct 15, 2025

Uh oh!

gnprice commented Oct 16, 2025

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vinodsingh-dev-gsoc commented Sep 30, 2025 •

edited

Loading

vinodsingh-dev-gsoc commented Oct 8, 2025 •

edited

Loading